Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Developer demo of alternate wiki store implementation #7329

Draft
wants to merge 76 commits into
base: master
Choose a base branch
from

Conversation

Jermolene
Copy link
Owner

@Jermolene Jermolene commented Mar 4, 2023

This PR introduces two alternate implementations of the tiddler store object:

  • A smallest possible plain JS wiki store implementation, sharing as much implementation as possible with the existing core implementation with as little code duplication as possible
  • An implementation based on the WebAssembly build of sqlite3
    • Wasm usage from a file URI without requiring external files
    • Sql implementations of main wiki store methods
    • Pass test suite with sqlite3 store
    • Tag indexer optimisations
    • Explore further optimisations

Testing with the current prerelease wiki, the sqlite3 implementation is currently significantly slower than the standard plain JavaScript wiki store implementation in boot.js.

  • Plain JS store:
    • mainRender: 89.20ms
    • styleRefresh: 4.00ms
    • mainRefresh: 12.00ms
  • sqlite3 store:
    • mainRender: 300.00ms
    • styleRefresh: 380.30ms
    • mainRefresh: 175.80ms

This is the very barebones beginnings of a demo implementation of an alternate tiddler store. It is not functional. If using the Vercel builds, open developer tools in the browser to see it failing due to the absence of basic wiki methods.

The plan is to build it up into the smallest possible plain JS wiki store implementation, sharing as much implementation as possible with the existing core implementation with as little code duplication as possible. It could then serve as the basis for future experiments with wiki stores based on SQLite (@linonetwo), or a custom append only database (@yaisog).
@vercel
Copy link

vercel bot commented Mar 4, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
tiddlywiki5 ✅ Ready (Inspect) Visit Preview Dec 11, 2023 8:43pm

boot/boot.js Outdated
@@ -1466,10 +1466,10 @@ $tw.Wiki = function(options) {
// Dummy methods that will be filled in after boot
$tw.Wiki.prototype.clearCache =
$tw.Wiki.prototype.clearGlobalCache =
$tw.Wiki.prototype.enqueueTiddlerEvent = function() {};
$tw.Wiki.prototype.enqueueTiddlerEvent = $tw.Wiki.prototype.enqueueTiddlerEvent || function() {};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change and all the following in this file so these methods can be optionally be defined in the code of the alternate store implementation?

The comment says they "will be filled in after boot". Won't that overwrite the method if it is implemented in the alternate store?

Also with the above it looks like clearCache and clearGlobalCache will also have the same function as enqueueTiddlerEvent if it has an alternate store implementation. Is that intended?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says they "will be filled in after boot". Won't that overwrite the method if it is implemented in the alternate store?

These three methods all have an implementation loaded from wiki.js, but need to be invoked before wiki.js is fully loaded, so we provide this dummy implementation.

Is this change and all the following in this file so these methods can be optionally be defined in the code of the alternate store implementation?

That's the idea. At the moment the additional methods in wiki.js are loaded unconditionally, and so can't be overridden by alternate stores, which is OK for the moment but might need to be revisited

Also with the above it looks like clearCache and clearGlobalCache will also have the same function as enqueueTiddlerEvent if it has an alternate store implementation. Is that intended?

These methods are currently overwritten by the implementations in wiki.js, and as above there's currently no support for alternate stores to override those methods.

The wiki store implementation in boot.js has always used a closure for key state information like the hashmap of tiddlers and shadow tiddlers, which makes clear which wiki methods actually have and need access to the raw tiddler store, and to encapsulate the implementation details of the store. That means that it should be possible to just override those methods that have access to the private data in the closure, and seamlessly take advantage of the implementation of the higher level methods in wiki.js.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable and I might still be missing part of your point.

By my thinking these 3 lines should have either remained unchanged since overwriting them would only last until wiki.js is loaded and therefore isn't very useful:

$tw.Wiki.prototype.clearCache =
$tw.Wiki.prototype.clearGlobalCache =
$tw.Wiki.prototype.enqueueTiddlerEvent = function() {};

or each should have their own separate overwrite:

$tw.Wiki.prototype.clearCache = $tw.Wiki.prototype.clearCache || function();
$tw.Wiki.prototype.clearGlobalCache = $tw.Wiki.prototype.clearGlobalCache || function();
$tw.Wiki.prototype.enqueueTiddlerEvent = $tw.Wiki.prototype.enqueueTiddlerEvent || function();

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @btheado I think you're right that there's no point in providing an underride for these methods because they will be overwritten later.

@pmario
Copy link
Contributor

pmario commented Mar 5, 2023

I always thought, that the TW concept could have "additional" $tw.wiki stores if needed. ...

With the implementation like this there can only be 1 type of store. Is this intended or is it just an experiment?

@linonetwo
Copy link
Contributor

In plugin, we always call $tw.wiki, so a singleton is better.

Just need some way to hook into existing $tw.wiki

@Jermolene
Copy link
Owner Author

I always thought, that the TW concept could have "additional" $tw.wiki stores if needed. ...

I think that's an orthogonal goal, which would be accomplished by reclassifying the existing $tw.wiki instance as the root admin wiki, and allow for a dynamic set of child wikis that can be created and deleted without a restart.

With the implementation like this there can only be 1 type of store. Is this intended or is it just an experiment?

An alternate store along these lines could still expose its own constructor so that further instances could be created.

@joshuafontany
Copy link
Contributor

Thanks for starting to sketch this out. I think this would allow tighter integration between TW and data-stores like the Yjs CRDT document(s). :)

@linonetwo
Copy link
Contributor

About how to use sqlite and if it should be a plugin #7531

And make the demo storage areas switchable

@joshuafontany's implementation was in #7521
@linonetwo
Copy link
Contributor

How to support multiple plugins?

I want to create a sqlite plugin that works on nodejs server side, loading tiddler and indexer from sqlite instead of fs, then do swr from fs.
I also want to create a plugin that work on client side, to get indexer&tiddlers using ipc, so we can skip most of parsing on client booting.
And there might be a plugin you want to create, work on client side, using wasm sqlite to cache content...

@Jermolene
Copy link
Owner Author

Hi @linonetwo the "demo-alternate-store" plugin in this PR is intended to be a starting point for people wanting to write their own custom alternate store plugins. The current implementation only works in the browser but will be extended to work under Node.js as well.

To support multiple alternate store plugins being loaded at the same time we will also need a mechanism for selecting between them.

…pendencies

We get a reference to sqlite3 but we're not yet doing anything with it

Also note that this approach leads to duplication - there will be two copies of sqlite3.js and sqlite3.wasm in each generated HTML file. The plan is to dynamically retrieve those tiddlers from the store area rather than baking them into the raw markup area
empty.html with the plugin is now 4.1MB
Previously, we were using the existing addIndexer method to piggyback adding our own internal indexers.
At the moment the optimiser returns a list of chainable functions, it would be simpler to just return a single function
operation.suffixes.push([]);
$tw.utils.each(subsuffix.split(","),function(entry) {
entry = $tw.utils.trim(entry);
if(entry) {
operation.suffixes[operation.suffixes.length -1].push(entry);
}
});
});
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the indent-level here is wrong

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @pmario fixed in 4897248

};
$tw.utils.evalSandboxed(sqlite3js,context,"$:/plugins/tiddlywiki/sqlite3store/sqlite3.js",true);
// Create a Blob URL for the wasm data
var sqlite3wasm = thisPluginTiddlers["$:/plugins/tiddlywiki/sqlite3store/sqlite3.wasm"].text;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this, instead of $tw.wiki.getTiddlerText ?

Context is #7947

$tw.utils.evalSandboxed(sqlite3js,context,"$:/plugins/tiddlywiki/sqlite3store/sqlite3.js",true);
// Create a Blob URL for the wasm data
var sqlite3wasm = thisPluginTiddlers["$:/plugins/tiddlywiki/sqlite3store/sqlite3.wasm"].text;
var decodedData = window.atob(sqlite3wasm),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get

Uncaught InvalidCharacterError: Failed to execute 'atob' on 'Window': The string to be decoded contains characters outside of the Latin1 range.

when mimicking your usage.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@linonetwo, notice Jeremy's tiddlywiki.info is encoding the file as base64. The atob can't decode it if it wasn't encoded.

		{
			"file": "sqlite3.wasm",
			"encoding": "base64",
			"fields": {
				"type": "application/wasm",
				"title": "$:/plugins/tiddlywiki/sqlite3store/sqlite3.wasm"
			}
		}

Copy link
Contributor

@linonetwo linonetwo Jan 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@btheado I have a few tries, even I change encoding to utf8, when I use $tw.wiki.getTiddlerText, I always get the base64 encoded test. Same even set "type": "text/text".

I tried this because 30MB wasm become 80MB of string after become a tiddler and I want to reduce it.

(Update: fixed by #7948)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, TiddlyWiki doesn't currently support true binary tiddlers; we only support base64 encoded pseudo-binary tiddlers.

@@ -2438,6 +2471,7 @@ $tw.boot.initStartup = function(options) {
$tw.utils.registerFileType("image/svg+xml","utf8",".svg",{flags:["image"]});
$tw.utils.registerFileType("image/vnd.microsoft.icon","base64",".ico",{flags:["image"]});
$tw.utils.registerFileType("image/x-icon","base64",".ico",{flags:["image"]});
$tw.utils.registerFileType("application/wasm","base64",".wasm");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is important. Without this, wasm tiddler will be 2x larger, and can't properly loaded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants